Skip to content

Fix two ways a company-mode session becomes permanently unusable (#52) - #53

Open
evgemar wants to merge 2 commits into
HKUDS:mainfrom
evgemar:fix/delivery-review-republish-wedge
Open

Fix two ways a company-mode session becomes permanently unusable (#52)#53
evgemar wants to merge 2 commits into
HKUDS:mainfrom
evgemar:fix/delivery-review-republish-wedge

Conversation

@evgemar

@evgemar evgemar commented Aug 27, 2026

Copy link
Copy Markdown

Fixes both variants of the permanent session wedge reported in #52.

Two real company-mode runs, in two different projects, ended with a session that
could never accept another message:

Error: controller takeover found a mixed linked Task/WorkItem attempt envelope
for 'delivery::...'

Recovery required hand-editing SQLite, so the runs were effectively lost.

1. Do not republish a delivery review card over a settled WorkItem

The delivery-review Task closure and the WorkItem approval are two writes on two
different SQLite connections. An interruption between them leaves an
AWAITING_HUMAN Task next to an already approved WorkItem.
_ensure_open_final_delivery_review_checkpoints() then republishes the card and
re-parks the Task for good, and the pair fails the envelope check on every later
takeover.

Skip republishing when the linked WorkItem already reached a done phase. The new
helper is deliberately conservative: a missing link, a store without the
accessor, or any lookup failure reports False and preserves the previous
behaviour.

2. Treat a never-dispatched linked pair as symmetric, not mixed

settle_stale_delegation_run_claims_for_controller() selects work items by
claim, not by phase, so a card that was claimed but never dispatched still gets
inspected. Both sides then carry an entirely empty attempt envelope, which failed
every branch: the raise rejects attempt_seq <= 0, and both escape hatches
require either attempt_seq > 0 or a terminal phase.

An empty envelope on both sides is symmetric — there is nothing to reconcile.
The remaining conjuncts already pin project, run, link kind, projection id and
claim identity, and linked_task_status == expected_linked_task_status proves
the pair agrees under task_status_for_phase. Requiring DONE_PHASES on top of
that made a consistent waiting_dependencies / blocked pair raise.

The guard itself is untouched for genuinely mixed envelopes — the existing tests
that assert the raise still pass.

Tests

tests/test_claim_release_invariant.py gains
test_controller_takeover_skips_never_dispatched_consistent_pair, which fails on
main with the exact production error and passes with this change.

Full suite: 2786 passed. Nine failures are pre-existing on main in this
environment (macOS permission errors on chflags in temp dirs); the failure set
is byte-identical with and without this branch.

Not addressed here

The underlying non-atomicity remains: the Task closure, the WorkItem approval and
the checkpoint terminal status are three separate writes, and the work-item
writer uses its own isolated connection, so they cannot share a transaction as
currently structured. That looks like an architectural decision rather than a
drive-by fix, and it is described in #52 for whoever picks it up.

Evgeny Marchenkov added 2 commits August 27, 2026 21:39
The delivery-review Task closure and the WorkItem approval are two writes
on two different SQLite connections, so an interruption between them can
leave an AWAITING_HUMAN Task next to an already approved WorkItem.

_ensure_open_final_delivery_review_checkpoints() then republishes the
card and re-parks the Task for good: the pair no longer satisfies the
attempt envelope check in settle_stale_delegation_run_claims_for_controller(),
so every later controller takeover raises and the session can never accept
another message.

Skip republishing when the linked WorkItem has already reached a done
phase. The helper is deliberately conservative — a missing link, a store
without the accessor, or any lookup failure reports False and preserves
the previous behaviour.

Refs HKUDS#52
settle_stale_delegation_run_claims_for_controller() selects work items by
claim, not by phase, so a card that was claimed but never dispatched is
inspected by controller takeover. Both sides then carry an empty attempt
envelope, which failed every branch: the raise condition rejects
attempt_seq <= 0, and both escape hatches require either attempt_seq > 0
or a terminal phase.

An empty envelope on both sides is symmetric, not mixed — there is
nothing to reconcile. The remaining conjuncts already pin project, run,
link kind, projection id and claim identity, and
linked_task_status == expected_linked_task_status proves the pair agrees
under task_status_for_phase. Requiring DONE_PHASES on top of that made a
consistent waiting_dependencies/blocked pair raise, and since the raise
happens on every later takeover the session could never be resumed.

Observed twice on real runs, in two different projects. Recovery required
hand-editing SQLite to clear the claim columns.

Refs HKUDS#52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant